Remove `rustc::existing_doc_keyword` lint by nnethercote · Pull Request #134202 · rust-lang/rust

nnethercote · 2024-12-12T07:29:07Z

The check doesn't require a lint.

nnethercote · 2024-12-12T07:29:18Z

Details in individual commits.

rustbot · 2024-12-12T07:29:19Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

rustbot · 2024-12-12T08:59:08Z

Some changes occurred in GUI tests.

cc @GuillaumeGomez

GuillaumeGomez · 2024-12-12T10:29:28Z

 }

-#[doc(keyword = "CookieMonster")]
+#[doc(keyword = "auto")]


Why this change?

The test now fails if the value isn't a valid keyword.

Well, that brings this question: why this change?

Previously the "is it a valid keyword?" test was done by the lint. It has now moved to check_doc_keyword and it runs in several test cases where the lint did not.

No I mean, doc(keyword) was voluntarily left "open" in what kind of keyword we could define. You changed this behaviour. That's the part I don't understand. I don't mind changing it away from being a lint.

The lint checked this:

fn is_doc_keyword(s: Symbol) -> bool { s <= kw::Union }

That hasn't changed (well, sym::SelfTy is now also accepted, for the exception). What has changed is how often this check is being applied.

I see, my bad.

GuillaumeGomez · 2024-12-12T10:29:38Z

 //@ !has "$.index[*][?(@.name=='hello')]"
 //@ !has "$.index[*][?(@.name=='bar')]"
-#[doc(keyword = "hello")]
+#[doc(keyword = "break")]


Why this change?

nnethercote · 2024-12-12T22:34:53Z

@GuillaumeGomez: the tests/rustdoc-gui/search-result-color.goml is driving me crazy. I can't understand how it works. The keyword used for it is CookieMonster. The docs for the keyword are given in tests/rustdoc-gui/src/test_docs/lib.rs and the test search is specified by tests/rustdoc-gui/search-result-keyword.goml.

It seems like I should just be able to change CookieMonster in those two files to any other identifier. In practice, some changes work and some don't. Some examples (all from a repo without this PR applied):

Run the test (x test tests/rustdoc-gui/search-result-color.goml): pass
Change CookieMonster to ZookieMonster in
tests/rustdoc-gui/src/test_docs/lib.rs and
tests/rustdoc-gui/search-result-keyword.goml, run test: fail (3 errors)
Change ZookieMonster back to CookieMonster, run test: fail (4 errors)
Delete build/x86_64-unknown-linux-gnu/test/, run test: pass
Change CookieMonster to CookieThief, run test: pass
Change CookieThief to Mookie, run test: fail (3 errors)
Delete build/x86_64-unknown-linux-gnu/test/, run test: fail (3 errors)
Change Mookie to Cook, run test: fail (4 errors)
Delete build/x86_64-unknown-linux-gnu/test/, run test: pass

Keywords that are similar to CookieMonster and start with C tend to work. Those that aren't similar fail. Sometimes deleting the test/ dir is required to get things working again after a failed attempt.

I have viewed the generated docs in a browser. They always work fine no matter what keyword I use, and the search always works as expected. So I'm baffled. I just want to change CookieMonster to any existing keyword (such as auto) to satisfy the keyword check. Any ideas what is happening with this test?

GuillaumeGomez · 2024-12-13T10:59:13Z

 // Waiting for the search results to appear...
 wait-for: "#search-tabs"
-assert-text: (".result-keyword .result-name", "keyword CookieMonster")
+assert-text: (".result-keyword .result-name", "keyword auto")


It's checking here the first search result. Considering that auto might not be the first item, we'll need to tweak it a bit.

I checked for that, auto is the first item. ZookieMonster was too, as were a bunch of other things I tried.

GuillaumeGomez · 2024-12-13T11:01:15Z

 // Checks that the "keyword" results have the expected text alongside them.
 go-to: "file://" + |DOC_PATH| + "/test_docs/index.html"
-write-into: (".search-input", "CookieMonster")
+write-into: (".search-input", "auto")


try:

Suggested change

write-into: (".search-input", "auto")

write-into: (".search-input", "keyword:auto")

Also you can open the generated docs with a web browser with build/[arch]/tests/rustdoc-gui/doc/lib2/index.html and test it manually.

I'll try the keyword change on Monday. I did already try testing manually in a browser, it always worked fine no matter what I tried. That's why it was so baffling why some things worked but most didn't -- I could find an explanation to explain the success/failure differences.

GuillaumeGomez · 2024-12-13T13:47:19Z

Found the issue in search-result-color.goml: there is no keyword displayed, meaning the test will fail since we can't check the color for a keyword. We need to use another keyword that might match other items.

GuillaumeGomez · 2024-12-13T13:58:34Z

I pushed a fix to your branch. To keep the fo query working, I used the for keyword instead.

nnethercote · 2024-12-15T21:48:50Z

Thank you! It was the "/test_docs/index.html?search=coo" that I overlooked. That explains why CookieMonster and CookieThief and similar terms worked, but other terms didn't.

nnethercote · 2024-12-15T21:50:24Z

I'm happy for this to merge if you are.

GuillaumeGomez · 2024-12-16T00:00:36Z

I am too so let's go!

@bors r=GuillaumeGomez,nnethercote

bors · 2024-12-16T00:00:39Z

📌 Commit bdbaeae has been approved by GuillaumeGomez,nnethercote

It is now in the queue for this repository.

…d, r=GuillaumeGomez,nnethercote Remove `rustc::existing_doc_keyword` lint The check doesn't require a lint. r? `@GuillaumeGomez`

…llaumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#134111 (Fix `--nocapture` for run-make tests) - rust-lang#134202 (Remove `rustc::existing_doc_keyword` lint) - rust-lang#134314 (Make sure to use normalized ty for unevaluated const in default struct value) - rust-lang#134329 (Add m68k_target_feature) - rust-lang#134342 (crashes: more tests) - rust-lang#134354 (Handle fndef rendering together with signature rendering) Failed merges: - rust-lang#133265 (Add a range argument to vec.extract_if) r? `@ghost` `@rustbot` modify labels: rollup

…d, r=GuillaumeGomez,nnethercote Remove `rustc::existing_doc_keyword` lint The check doesn't require a lint. r? ``@GuillaumeGomez``

…d, r=GuillaumeGomez,nnethercote Remove `rustc::existing_doc_keyword` lint The check doesn't require a lint. r? ```@GuillaumeGomez```

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#134202 (Remove `rustc::existing_doc_keyword` lint) - rust-lang#134354 (Handle fndef rendering together with signature rendering) - rust-lang#134368 (Use links to edition guide for edition migrations) - rust-lang#134371 (Check for array lengths that aren't actually `usize`) - rust-lang#134378 (An octuple of polonius fact generation cleanups) Failed merges: - rust-lang#134365 (Rename `rustc_mir_build::build` to `builder`) r? `@ghost` `@rustbot` modify labels: rollup

aDotInTheVoid

@bors r- until rustdoc-json text fixed.

aDotInTheVoid · 2024-12-16T23:28:39Z

@bors r-

All the other unconditional keywords are in the alphabetical order, but `while` is for some reason not.

`CheckAttrVisitor::check_doc_keyword` checks `#[doc(keyword = "..")]` attributes to ensure they are on an empty module, and that the value is a non-empty identifier. The `rustc::existing_doc_keyword` lint checks these attributes to ensure that the value is the name of a keyword. It's silly to have two different checking mechanisms for these attributes. This commit does the following. - Changes `check_doc_keyword` to check that the value is the name of a keyword (avoiding the need for the identifier check, which removes a dependency on `rustc_lexer`). - Removes the lint. - Updates tests accordingly. There is one hack: the `SelfTy` FIXME case used to used to be handled by disabling the lint, but now is handled with a special case in `is_doc_keyword`. That hack will go away if/when the FIXME is fixed. Co-Authored-By: Guillaume Gomez <guillaume1.gomez@gmail.com>

nnethercote · 2024-12-17T02:56:57Z

@bors r=GuillaumeGomez

bors · 2024-12-17T02:56:59Z

📌 Commit 121e87b has been approved by GuillaumeGomez

It is now in the queue for this repository.

rustbot assigned GuillaumeGomez Dec 12, 2024

This comment has been minimized.

Sign in to view

nnethercote force-pushed the rm-existing_doc_keyword branch from 7a0a3f2 to 353aff4 Compare December 12, 2024 08:59

This comment has been minimized.

Sign in to view

GuillaumeGomez reviewed Dec 12, 2024

View reviewed changes

Comment thread tests/ui/internal-lints/existing_doc_keyword.rs

GuillaumeGomez reviewed Dec 13, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 16, 2024

GuillaumeGomez mentioned this pull request Dec 16, 2024

Rollup of 6 pull requests #134359

Closed

matthiaskrgr mentioned this pull request Dec 16, 2024

Rollup of 5 pull requests #134380

Closed

aDotInTheVoid reviewed Dec 16, 2024

View reviewed changes

Comment thread tests/rustdoc-json/keyword.rs Outdated

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 16, 2024

nnethercote and others added 2 commits December 17, 2024 13:40

Move doc(keyword = "while").

f10169c

All the other unconditional keywords are in the alphabetical order, but `while` is for some reason not.

nnethercote force-pushed the rm-existing_doc_keyword branch from bdbaeae to 121e87b Compare December 17, 2024 02:56

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 17, 2024

jhpratt mentioned this pull request Dec 17, 2024

Rollup of 9 pull requests #134411

Closed

jhpratt mentioned this pull request Dec 17, 2024

Rollup of 10 pull requests #134414

Merged

bors merged commit 52b4557 into rust-lang:master Dec 17, 2024

rustbot added this to the 1.85.0 milestone Dec 17, 2024

nnethercote deleted the rm-existing_doc_keyword branch December 17, 2024 21:24

	write-into: (".search-input", "auto")
	write-into: (".search-input", "keyword:auto")

Uh oh!

Conversation

nnethercote commented Dec 12, 2024

Uh oh!

nnethercote commented Dec 12, 2024

Uh oh!

rustbot commented Dec 12, 2024

Uh oh!

This comment has been minimized.

rustbot commented Dec 12, 2024

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nnethercote commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Dec 13, 2024

Uh oh!

GuillaumeGomez commented Dec 13, 2024

Uh oh!

nnethercote commented Dec 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nnethercote commented Dec 15, 2024

Uh oh!

GuillaumeGomez commented Dec 16, 2024

Uh oh!

bors commented Dec 16, 2024

Uh oh!

aDotInTheVoid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aDotInTheVoid commented Dec 16, 2024

Uh oh!

nnethercote commented Dec 17, 2024

Uh oh!

bors commented Dec 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nnethercote commented Dec 12, 2024 •

edited

Loading

nnethercote commented Dec 15, 2024 •

edited

Loading